New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

OIDC make token iat claim optional via config property #39249

Merged

sberyozkin merged 1 commit into quarkusio:main from wesleysalimansdvb:39215-oidc-make-age-optional

Mar 8, 2024

Contributor

wesleysalimansdvb commented Mar 7, 2024 •

edited by geoand

Loading

Closes: No way to configure iat claim as optional in quarkus-oidc #39215

quarkus-bot bot added the area/oidc label

michalvavrik requested a review from sberyozkin

March 7, 2024 10:55

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java Outdated Show resolved Hide resolved

sberyozkin reviewed

View reviewed changes

Member

sberyozkin left a comment

@wesleysalimansdvb Thanks for the PR, I'd like to propose to have it named everywhere as issuedAtRequired and also document that this property will be ignored for the ID token verification - in OidcIdentityProvider there must be a false value passed when ID token is being verified.

Also, please update OidcRecorder to throw a configuration exception if this property is true and the quarkus.oidc.token-age is also true.

Thanks

Contributor Author

wesleysalimansdvb commented Mar 7, 2024

@sberyozkin I processed your feedback :)

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java Outdated Show resolved Hide resolved

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/OidcTenantConfig.java Outdated Show resolved Hide resolved

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcIdentityProvider.java Outdated Show resolved Hide resolved

Contributor Author

wesleysalimansdvb commented Mar 7, 2024

@sberyozkin thanks for the good feedback, I learned a lot by contributing to the quarkus codebase!

sberyozkin approved these changes

View reviewed changes

Member

sberyozkin left a comment

Thanks @wesleysalimansdvb, I'll suggest a few more tiny updates, and we'll merge, please squash the commits if you don't mind

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java Outdated Show resolved Hide resolved

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcProvider.java Outdated Show resolved Hide resolved

sberyozkin reviewed

View reviewed changes

extensions/oidc/runtime/src/main/java/io/quarkus/oidc/runtime/OidcRecorder.java Outdated Show resolved Hide resolved

sberyozkin approved these changes

View reviewed changes

Member

sberyozkin left a comment

@wesleysalimansdvb Very last (I promise :-)) 3 suggestions are proposed, thanks


          oidc make token age optional via config property

4d825af

update configuration exception and rename variable

Rewording documentation and always check id token iat

oidc rename variable name throw configuration property error

wesleysalimansdvb force-pushed the 39215-oidc-make-age-optional branch from 8b9f85c to 4d825af Compare

March 8, 2024 12:42

Contributor Author

wesleysalimansdvb commented Mar 8, 2024 •

edited

Loading

@sberyozkin Squashed the commits in to 1 commit, I hope everything is alright like this. If not, I'll see :p

Thanks!

sberyozkin approved these changes

View reviewed changes

quarkus-bot bot commented Mar 8, 2024 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit 4d825af.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

sberyozkin merged commit fb3c0b8 into quarkusio:main

22 checks passed

quarkus-bot bot added this to the 3.9 - main milestone

quarkus-bot bot added the kind/enhancement label

sberyozkin added the triage/backport-3.8 label

Member

sberyozkin commented Apr 4, 2024

Proposing a backport to 3.8 following a request from @madocx at #39887, as the changes do not seem to be sensitive

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/oidc kind/enhancement triage/backport-3.8